SafeAppointment type actually doesn't have this properties, so I have removed them

Then we should remove SafeAppointment? Now there is no point in having it

We talked about this in our meeting — in the Scheduler codebase "Appointment" is the source data (what user creates). What we render on the view is an "Occurrence" — we already have the Occurrence type and getOccurrences() method for this.

This is not a nitpick. This is the wrong name. These classes render items on a view, not the source data. Right now we have SafeAppointment, AppointmentItemViewModel, BaseAppointment — three different things all called "Appointment". This makes the code harder to read.

Please rename to BaseOccurrenceView / GridOccurrenceView / AgendaOccurrenceView, or at least AppointmentView.

Applied new AppointmentView name

I looked at what DOMComponent features are actually used here:

• $element() — just a jQuery ref
• option() — just a props object
• _getTemplateByOption() — one call: template.render({ container, model }). Parent can do this.
• _createActionByOption() — could be a direct callback

The only place that really needs DOMComponent is _createComponent in AppointmentCollector — it creates a Button and handles disabled/rtl cascading.

But that does not mean every appointment needs to be a DOMComponent. The Collector can get a button factory from the parent instead.

Also: focus() is an empty stub. We lost KBN and accessibility that CollectionWidget gave us for free.

I think each appointment should be a simple object (jQuery element + position + classes), not a full DOMComponent. Template rendering should stay in the parent — like how CollectionWidget does it for List, Gallery, TileView.

Very good point, but I would suggest to postpone removal of DOMComponent. But right now it's a familiar mechanism to pass properties and use common functions.

DOMComponent doesn't introduce a lot of overheads to the appointments and I believe it will be easy to remove DOMComponent in the future.

If you don't mind I would postpone this

The test for "show correct title" (line 95-105) needs:

1. getGridAppointmentProperties() → mockGridViewModel() (40 lines, 15+ fields)
2. getMockedBaseAppointmentProperties() → mocked ResourceManager, DataAccessor
3. createGridAppointment() → DOMComponent + @ts-expect-error + await process.nextTick

All this to check that a div has text "Test title".

Simpler version:

const $el = renderAppointment({ text: "Test title", startDate, endDate });
expect($el.find(".dx-scheduler-appointment-title").text()).toBe("Test title");

The rendering logic is ~30 lines of jQuery. But we need a 126-line mock file to test it. These tests check how the component is connected (DOMComponent lifecycle, template manager, resource manager) — not what it does (render a title in a div). If AppointmentItemViewModel adds a new field, the mock breaks even though title rendering did not change.

I think this happened because AppointmentItemViewModel is too complex and has too many things inside. It mixes rendering data, positioning, view state, and data access all in one object. If the view model was simpler and focused only on what the component needs to render, the tests would be simpler too.

Simplified properties that are passed to the appointments or collector. Now mocks in this tests are simpler.

minor: Just a thought — this is classic LCS with O(n*m) time and space. With virtual scrolling we can have 50-100+ visible appointments, and this runs on every scroll.

Since appointments use absolute positioning (top/left), DOM order does not matter. A simple Map<itemData, component> would give O(n+m) — iterate both lists, find added/removed/kept.

Not critical, you can keep it as is. But think about it — a map-based approach would be simpler and faster here because we do not need to preserve order.

I also thought about this solution, but we can't use this approach, because recurring appointments have the same itemData, so we don't have a unique key for the appointments

minor: This removes .dx-item-content from the selector, which changes CSS specificity. This is the only change that affects production (old implementation). The etalon screenshots changed — so there is a visual diff.

This feels out of place in this PR. Maybe move to a separate small PR so we can test it on its own?

I am not sure if the screenshots changed because of this, but I would like to know if it was because of css or not.

@sjbur definitely screenshots changed because of this. But it seems that now it's better, because all day appts have the same padding-right as the common appts.

That happened because of .dx-item-content selector, some styles weren't applied to the all day appts.

Memory leak: appointmentBySortIndex is never reset before re-rendering. Each call to renderAgendaAppointments appends new entries but stale references from previous renders remain. Add this.appointmentBySortIndex = {}; before the forEach.

Done

Only $commonContainer is cleared here but $allDayContainer is not. If a previous non-agenda render placed elements into $allDayContainer, switching to agenda view will leave those orphaned DOM nodes. Add this.$allDayContainer?.empty(); here as well.

Thank you for notificing, I have applied the fix and added a test

minor: instanceof EmptyTemplate check looks fragile. Maybe better to use flag or null check? Not blocker.

EmptyTemplate is provided by default:

If user sets custom template function, then it should be applied.

If user sets null or undefined, then template should be empty according to the old impl (codepen).

If default templates is null or undefined, we would need to somehow distinguish between users null or undefined and default null or undefined.

So I think current way is the simplest to support custom templates.

minor: No test for grid → agenda switch with allDay appointments. Good to have for cleanup verification.

Added the test

Can we move those classes this to AGENDA_APPOINTMENT_CLASSES?

Then we should remove SafeAppointment? Now there is no point in having it

If you specify "format" argument in createFormattedDateText as of type DateFormatType, then we don't need to cast formatType as any.

This will break for collectors.

When a collector aggregates all-day appointments, it gets allDay: true from the view model. So it goes into allDayFragment → $allDayContainer. But collectors are compact overflow buttons with absolute positioning — they should always go into $commonContainer.

const fragment = (allDay && !isAppointmentCollectorViewModel(diffItem.item))
  ? allDayFragment : commonFragment;

Ok, thank you. I will fix this in the future. I am sure, that not all of the cases are covered in this PR, so I expect that we will notice more and more such cases and fix them pointwise

This assigns numeric width/height to geometry. But AgendaAppointmentViewModel.width is a string ('100%').

Right now agenda does not enter this path — it uses renderAgendaAppointments. But if it ever does, this will silently pass wrong types to resize().

I can't imagine a situation when it could ever go this way. But I can suggest to implement a test to cover this case. What do you think?

You're right, this path is unreachable with the current code. Let's skip it

This takes (newViewModel, oldViewModel), but the utility getViewModelDiff takes (oldViewModel, newViewModel). You swap them inside.

This is confusing. If someone reads this method signature and calls the utility directly — they will swap old/new. Consider matching the utility order (old, new) here too.

I agree, looks like small refactoring, but I will apply it in the future PR